Skip to content

feat(sandbox): auto-detect TLS and terminate unconditionally for credential injection#544

Merged
johntmyers merged 8 commits intomainfrom
feat/533-auto-tls-termination
Mar 24, 2026
Merged

feat(sandbox): auto-detect TLS and terminate unconditionally for credential injection#544
johntmyers merged 8 commits intomainfrom
feat/533-auto-tls-termination

Conversation

@johntmyers
Copy link
Copy Markdown
Collaborator

Summary

The sandbox proxy now auto-detects TLS by peeking the first bytes of every allowed connection. When TLS is detected, it terminates unconditionally -- enabling credential injection and optional L7 inspection without requiring explicit tls: terminate in the policy.

Related Issue

Closes #533

UX Changes

Before: Users had to add tls: terminate to each HTTPS endpoint for L7 inspection and credential injection to work. Missing it caused silent 401s or protocol mismatch errors.

After: TLS termination is automatic. Users write L4 rules (host + port) and get credential injection for free. L7 rules add request-level enforcement on top.

  • tls: terminate / tls: passthrough -- deprecated (log warning), treated as auto-detect
  • tls: skip -- new explicit opt-out for raw tunneling
  • No breaking changes for existing policies

Changes

  • l7/tls.rs -- looks_like_tls() detection (TLS ClientHello peek)
  • l7/mod.rs -- TlsMode::Auto/Skip, parse_tls_mode(), deprecation warnings, validation updates
  • l7/relay.rs -- relay_passthrough_with_credentials() for auto-terminated L4 traffic
  • proxy.rs -- Restructured: peek -> auto-detect -> terminate/relay. Fixed CONNECT log message.
  • sandbox-policy.rego -- endpoint_has_extended_config matches tls field
  • mechanistic_mapper.rs -- Removed auto-generated tls: terminate
  • scripts/smoke-test-network-policy.sh -- 8 e2e tests (L4, L7, cred injection, tls:skip)
  • 13 doc files updated (user docs, architecture, skill examples)

Testing

  • 291 unit tests passing
  • Smoke test 8/8 passing
  • E2E (running in CI)

Checklist

  • Follows Conventional Commits
  • Architecture docs updated
  • User-facing docs updated

@johntmyers johntmyers requested a review from a team as a code owner March 23, 2026 04:46
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Mar 23, 2026
@johntmyers johntmyers self-assigned this Mar 23, 2026
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Mar 23, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 23, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-03-24 01:45 UTC

…ential injection

Closes #533

The proxy now auto-detects TLS by peeking the first bytes of each
connection. When TLS is detected, it terminates unconditionally —
enabling credential injection and optional L7 inspection without
requiring explicit 'tls: terminate' in the policy.

- Add looks_like_tls() detection (TLS ClientHello: 0x16 0x03 0x0N)
- Replace TlsMode::Passthrough/Terminate with Auto/Skip
- Restructure proxy dispatch: peek → auto-detect → terminate/relay
- Add relay_passthrough_with_credentials for terminate-but-no-L7 path
- Deprecate 'tls: terminate' and 'tls: passthrough' (log warnings)
- Add 'tls: skip' as explicit opt-out escape hatch
- Update validation, tests, docs, examples, and skill references
…dependent query

The tls field was only parsed when an L7 protocol was configured,
so tls: skip on L4-only endpoints was silently ignored. Fix:
- Add parse_tls_mode() independent of parse_l7_config()
- Add query_tls_mode() in proxy.rs
- Add endpoint_has_extended_config rule for tls field in Rego
- Update smoke test to verify credential injection and tls: skip
@johntmyers johntmyers force-pushed the feat/533-auto-tls-termination branch from b88a1b1 to 034617c Compare March 24, 2026 00:36
pimlock
pimlock previously approved these changes Mar 24, 2026
Comment thread .agents/skills/generate-sandbox-policy/examples.md Outdated
@johntmyers johntmyers merged commit f37b69b into main Mar 24, 2026
10 checks passed
@johntmyers johntmyers deleted the feat/533-auto-tls-termination branch March 24, 2026 01:45
laitingsheng added a commit to NVIDIA/NemoClaw that referenced this pull request Apr 20, 2026
OpenShell [v0.0.15+ auto-terminates TLS unconditionally](NVIDIA/OpenShell#544)
on every detected TLS stream, which also applies to WSS. Without an
explicit opt-out, messaging WebSocket endpoints (previously shipping
as pure L4 CONNECT tunnels via `access: full`) are now MITMed, and
the Discord client consistently loses its gateway connection with
close code 1006.

Add `tls: skip` to the affected endpoints to restore the pre-v0.0.15
pass-through behaviour:
  - discord preset: gateway.discord.gg
  - slack preset:   wss-primary.slack.com, wss-backup.slack.com
  - baseline openclaw-sandbox.yaml: gateway.discord.gg

Extend the policy schemas to accept `skip` in the `tls` enum.

Fixes #2092.
Fixes #1738.

Related to #2085.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
ericksoa pushed a commit to NVIDIA/NemoClaw that referenced this pull request Apr 20, 2026
OpenShell [v0.0.15+ auto-terminates TLS
unconditionally](NVIDIA/OpenShell#544) on every
detected TLS stream, which also applies to WSS. Without an explicit
opt-out, messaging WebSocket endpoints (previously shipping as pure L4
CONNECT tunnels via `access: full`) are now MITMed, and the Discord
client consistently loses its gateway connection with close code 1006.

Add `tls: skip` to the affected endpoints to restore the pre-v0.0.15
pass-through behaviour:
  - discord preset: gateway.discord.gg
  - slack preset:   wss-primary.slack.com, wss-backup.slack.com
  - baseline openclaw-sandbox.yaml: gateway.discord.gg

Extend the policy schemas to accept `skip` in the `tls` enum.

Fixes #2092.
Fixes #1738.

Related to #2085.

<!-- markdownlint-disable MD041 -->
## Summary
<!-- 1-3 sentences: what this PR does and why. -->

## Related Issue
<!-- Fixes #NNN or Closes #NNN. Remove this section if none. -->

## Changes
<!-- Bullet list of key changes. -->

## Type of Change

- [X] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. -->
- [X] `npx prek run --all-files` passes
- [X] `npm test` passes
- [ ] Tests added or updated for new or changed behavior
- [X] No secrets, API keys, or credentials committed
- [ ] Docs updated for user-facing behavior changes
- [ ] `make docs` builds without warnings (doc changes only)
- [ ] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

## AI Disclosure
<!-- If an AI agent authored or co-authored this PR, check the box and
name the tool. Remove this section for fully human-authored PRs. -->
- [X] AI-assisted — tool: Claude Code<!-- e.g., Claude Code, Cursor,
GitHub Copilot -->

---
<!-- DCO sign-off required by CI. Run: git config user.name && git
config user.email -->
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved TLS handling for Discord and Slack WebSocket connections to
prevent timeout issues and restore proper pass-through behavior for
long-lived secure connections.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(sandbox): auto-detect TLS and terminate unconditionally for credential injection

2 participants